[clang-doc] Add --asset option to clang-doc #94717
Conversation
|
@llvm/pr-subscribers-clang-tools-extra Author: None (PeterChou1) ChangesAdds a new option --asset which allows users to specified the asset folder for the html output of clang-doc Full diff: https://github.com/llvm/llvm-project/pull/94717.diff 1 Files Affected:
diff --git a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
index 21b581fa6df2e..df53c46b4a76e 100644
--- a/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
+++ b/clang-tools-extra/clang-doc/tool/ClangDocMain.cpp
@@ -81,6 +81,12 @@ static llvm::cl::list<std::string> UserStylesheets(
llvm::cl::desc("CSS stylesheets to extend the default styles."),
llvm::cl::cat(ClangDocCategory));
+static llvm::cl::opt<std::string>
+ UserAssetPath("asset",
+ llvm::cl::desc("User supplied asset path for html output to "
+ "override the default css and js files"),
+ llvm::cl::cat(ClangDocCategory));
+
static llvm::cl::opt<std::string> SourceRoot("source-root", llvm::cl::desc(R"(
Directory where processed files are stored.
Links to definition locations will only be
@@ -131,12 +137,54 @@ std::string GetExecutablePath(const char *Argv0, void *MainAddr) {
return llvm::sys::fs::getMainExecutable(Argv0, MainAddr);
}
+void GetAssetFiles(clang::doc::ClangDocContext CDCtx) {
+ std::error_code Code;
+ for (auto DirIt = llvm::sys::fs::directory_iterator(
+ std::string(UserAssetPath), Code),
+ dir_end = llvm::sys::fs::directory_iterator();
+ !Code && DirIt != dir_end; DirIt.increment(Code)) {
+ llvm::SmallString<128> filePath = llvm::SmallString<128>(DirIt->path());
+ if (llvm::sys::fs::is_regular_file(filePath)) {
+ if (filePath.ends_with(".css")) {
+ CDCtx.UserStylesheets.push_back(std::string(filePath));
+ } else if (filePath.ends_with(".js")) {
+ CDCtx.FilesToCopy.push_back(std::string(filePath));
+ }
+ }
+ }
+}
+
+void GetDefaultAssetFiles(const char *Argv0,
+ clang::doc::ClangDocContext CDCtx) {
+ void *MainAddr = (void *)(intptr_t)GetExecutablePath;
+ std::string ClangDocPath = GetExecutablePath(Argv0, MainAddr);
+ llvm::SmallString<128> NativeClangDocPath;
+ llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
+
+ llvm::SmallString<128> AssetsPath;
+ AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
+ llvm::sys::path::append(AssetsPath, "..", "share", "clang");
+ llvm::SmallString<128> DefaultStylesheet;
+ llvm::sys::path::native(AssetsPath, DefaultStylesheet);
+ llvm::sys::path::append(DefaultStylesheet,
+ "clang-doc-default-stylesheet.css");
+ llvm::SmallString<128> IndexJS;
+ llvm::sys::path::native(AssetsPath, IndexJS);
+ llvm::sys::path::append(IndexJS, "index.js");
+ CDCtx.UserStylesheets.insert(CDCtx.UserStylesheets.begin(),
+ std::string(DefaultStylesheet));
+ CDCtx.FilesToCopy.emplace_back(IndexJS.str());
+
+ llvm::outs() << "No default asset path found using default asset path: "
+ << AssetsPath << "\n";
+}
+
int main(int argc, const char **argv) {
llvm::sys::PrintStackTraceOnErrorSignal(argv[0]);
std::error_code OK;
const char *Overview =
- R"(Generates documentation from source code and comments.
+ R"(Generates documentation from source code and comments.
Example usage for files without flags (default):
@@ -182,23 +230,12 @@ Example usage for a project using a compile commands database:
{"index.js", "index_json.js"}};
if (Format == "html") {
- void *MainAddr = (void *)(intptr_t)GetExecutablePath;
- std::string ClangDocPath = GetExecutablePath(argv[0], MainAddr);
- llvm::SmallString<128> NativeClangDocPath;
- llvm::sys::path::native(ClangDocPath, NativeClangDocPath);
- llvm::SmallString<128> AssetsPath;
- AssetsPath = llvm::sys::path::parent_path(NativeClangDocPath);
- llvm::sys::path::append(AssetsPath, "..", "share", "clang");
- llvm::SmallString<128> DefaultStylesheet;
- llvm::sys::path::native(AssetsPath, DefaultStylesheet);
- llvm::sys::path::append(DefaultStylesheet,
- "clang-doc-default-stylesheet.css");
- llvm::SmallString<128> IndexJS;
- llvm::sys::path::native(AssetsPath, IndexJS);
- llvm::sys::path::append(IndexJS, "index.js");
- CDCtx.UserStylesheets.insert(CDCtx.UserStylesheets.begin(),
- std::string(DefaultStylesheet));
- CDCtx.FilesToCopy.emplace_back(IndexJS.str());
+ if (!UserAssetPath.empty() &&
+ llvm::sys::fs::is_directory(std::string(UserAssetPath))) {
+ GetAssetFiles(CDCtx);
+ } else {
+ GetDefaultAssetFiles(argv[0], CDCtx);
+ }
}
// Mapping phase
|
PeterChou1
changed the title
|
✅ With the latest revision this PR passed the C/C++ code formatter. |
PeterChou1
force-pushed
the
clang-doc-add-asset-option
branch
from
d535cb7 to
85581ed
Compare
| @@ -131,12 +137,55 @@ std::string GetExecutablePath(const char *Argv0, void *MainAddr) { | |||
| return llvm::sys::fs::getMainExecutable(Argv0, MainAddr); | |||
| } | |||
|
|
|||
| void GetAssetFiles(clang::doc::ClangDocContext &CDCtx) { | |||
| std::error_code Code; | |||
There was a problem hiding this comment.
nit: I'd use Err.
There isn't a completely consistent practice in Clang-Doc code, but FileErr seems most common. https://llvm.org/docs/ProgrammersManual.html#interoperability-with-std-error-code-and-erroror has a little to say, and the Error handling section on that page may be useful, but we don't have a firm convention.
There was a problem hiding this comment.
| std::error_code Code; | |
| std::error_code Err; |
| std::error_code Code; | |
| std::error_code FileErr; |
There was a problem hiding this comment.
This still uses Code. please use a naming convention consistent w/ this tool and other parts of LLVM before marking it resolved.
| // RUN: rm -rf %t | ||
| // RUN: mkdir %t |
There was a problem hiding this comment.
| // RUN: rm -rf %t | |
| // RUN: mkdir %t | |
| // RUN: rm -rf %t && mkdir %t |
There was a problem hiding this comment.
These still seem to be different steps. combining them is intentional. Actually, do you even need this directory? I don't think you do.
PeterChou1
force-pushed
the
clang-doc-add-asset-option
branch
from
5cba074 to
94b2979
Compare
| std::error_code Code; | ||
| llvm::SmallString<128> FilePath(UserAssetPath); | ||
| for (DirIterator DirIt = DirIterator(UserAssetPath, Code), | ||
| DirEnd = DirIterator(); |
There was a problem hiding this comment.
| DirEnd = DirIterator(); | |
| DirEnd; |
This doesn't need to be initialized that way, and appears to be the common pattern in clang and elsewhere when using directory_iterator. Usage should probably be consistent w/ what's used in other clang-tools-extra projects. If nothing else, there's an argument for being consistent w/ clang.
| // RUN: rm -rf %t | ||
| // RUN: mkdir %t |
There was a problem hiding this comment.
These still seem to be different steps. combining them is intentional. Actually, do you even need this directory? I don't think you do.
| @@ -131,12 +137,55 @@ std::string GetExecutablePath(const char *Argv0, void *MainAddr) { | |||
| return llvm::sys::fs::getMainExecutable(Argv0, MainAddr); | |||
| } | |||
|
|
|||
| void GetAssetFiles(clang::doc::ClangDocContext &CDCtx) { | |||
| std::error_code Code; | |||
There was a problem hiding this comment.
This still uses Code. please use a naming convention consistent w/ this tool and other parts of LLVM before marking it resolved.
| // CHECK: Using default asset: {{.*}}{{[\\/]}}share{{[\\/]}}clang |
There was a problem hiding this comment.
https://releases.llvm.org/14.0.0/docs/TestingGuide.html
See ‘pathsep’
There was a problem hiding this comment.
I think pathsep looks for : (or ; on Windows) it doesn't look for \ or /
PeterChou1
force-pushed
the
clang-doc-add-asset-option
branch
from
ca3d880 to
a9c8465
Compare
|
Looks Like this might break tests: http://45.33.8.238/linux/141118/step_7.txt Please take a look and revert for now if it takes a while to fix. |
Reverts #94717 This breaks on some buildbots: http://45.33.8.238/linux/141118/step_7.txt
Sorry for the late revert @nico. I'll have @PeterChou1 investigate before relanding. |
|
@nico Actually, I probably shouldn't have reverted. It seems like your GN build is incorrect, and isn't copying the asset files into the right place. The patch that updated that landed in #95187. @PeterChou1 is going to add you as a reviewer on the reland in question, but I think the existing premerge and post-submit bots are sufficient to show that this isn't something affecting the CMake build. |
Reapply #94717 Adds a new option --asset which allows users to specified the asset folder for the html output of clang-doc. This patch adds a better test for --asset option + fixes bug where clang-doc assumes that user supplied js file is assume to be index.js
Reapply llvm#94717 Adds a new option --asset which allows users to specified the asset folder for the html output of clang-doc. This patch adds a better test for --asset option + fixes bug where clang-doc assumes that user supplied js file is assume to be index.js
Adds a new option --asset which allows users to specify the asset folder for the html output of clang-doc.
Reverts llvm#94717 This breaks on some buildbots: http://45.33.8.238/linux/141118/step_7.txt
Adds a new option --asset which allows users to specified the asset folder for the html output of clang-doc
related to: #93928